feat(widget): add limit order mode#793
Conversation
🦋 Changeset detectedLatest commit: d0c4b6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
6f66b1a to
14cf8cf
Compare
58c0a26 to
4b18107
Compare
E2E Examples — all passedAll examples passed in the latest run. |
❌ E2E Dev Smoke — failed
1 passed · 3 failed · 0 skipped · 73s
View run — the full HTML report is attached as the |
E2E Playground resultsDetails
Failed testspreview › playground/settings.developer-controls.spec.ts › Playground settings — Developer controls (Loading preview) › restores the live widget when toggled off 📥 Download full HTML report (open the run → Artifacts → |
…omponent for limit order mode
…s for consistency
🔍 QA Review — EMB-322
🧠 What this ticket doesAdds a new "Limit" mode to the LI.FI widget — a bespoke feature for Jumper Pro's right-side order entry panel. The implementation introduces a mode toggle via
📋 Previously Flagged Items — Resolution Status
|
| # | Severity | Type | Issue |
|---|---|---|---|
| 1 | 🔴 Critical | Code | CI failures — _navigationTabs still uncommented in defaultWidgetConfig.ts |
🔴 [Critical] _navigationTabs still uncommented — CI is failing
packages/widget-playground/src/defaultWidgetConfig.ts HEAD (commit 326d11e1) still has:
_navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'],Neither post-review commit addressed this. The CI run against 8ba07dce (the most recent CI result) shows 19 Playground E2E failures and 3 Dev Smoke failures — all caused by the playground defaulting to a tab-bar widget instead of the plain Exchange view.
Developer's stated intent: "kept enabled on purpose for now to exercise the tab bar during testing; will restore before merge." That commitment is not yet fulfilled.
Fix: // _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit']. The limit tab is exercisable via the playground's mode panel without this being the global default.
✅ Resolved Items — Verification Notes
Item 2 — mergeDefaultFormValues fix (verified correct)
Final guard in createFormStore.ts:
value:
userValues[key]?.value !== undefined && userValues[key]?.value !== ''
? userValues[key]?.value
: defaultValues[key]?.value,partiallyFillable = false→false !== undefined && false !== ''= true → user value preserved ✅- numeric
0→ likewise preserved ✅ - empty string
''→ falls to default (intentional — commit326d11e1preserves text-field clearing semantics) ✅
The second commit correctly refines the first fix. No regression introduced.
Item 10 — aria-disabled keyboard accessibility (fully resolved)
EditablePriceChip is a styled Box (<div>) with no tabIndex and no role — not in the keyboard tab order. The inner PriceChipInput is a native <input> with disabled={disabled} — natively removed from tab order when disabled. CSS pointerEvents: none via '&[aria-disabled="true"]' blocks mouse interaction. The onClick guard is defence-in-depth. Fix is architecturally sound.
Items 3, 4, 7, 11 — Accepted with justification
- Item 3 (validUntil): SDK
@lifi/sdk@4.1.0processesvalidUntilbackend-side at route resolution; 60s refetch keeps drift negligible on a 1-day order. Justified. - Item 4 (success summary): No limit-specific success screen designed for this iteration. AC 13 remains open on ticket as a documented deferral. Justified.
- Item 7 (preset percentages):
[0, 1, 5, 10]confirmed as current design. Ticket text is outdated. Justified. - Item 11 (unit tests for hooks): No DOM/React test environment in widget package. Derivation math covered by
limitOrder.test.ts. Justified.
⚠️ Pre-merge Hygiene (2 items — no new QA cycle required)
- Item 8 — Public docs:
WidgetMode = 'limit'andWidgetEvent.NavigationTabChangedare new exported public types. PR checklist item unchecked. Complete before merge. - Item 12 — Visual showcase: PR body still contains
_TODO: attach a screen recording.... Complete before merge.
🆕 New Issues Introduced (0)
No new defects in the post-review commits. Changes are narrowly scoped to the flagged items.
🧪 Test Coverage
| Layer | Score | Files reviewed |
|---|---|---|
| Unit (Vitest) | Partial | packages/widget/src/utils/limitOrder.test.ts (pure utils covered), no hook/component tests |
| E2e (Playwright) | N/A | CI failures caused by Issue #1 — will clear once _navigationTabs is commented out |
Remaining unit gap (Low, carried from prior review):
applyPriceOffset(price, 0)(Market preset) — not tested. Low risk, but the branch is exercised by every Market button click.
🔗 Downstream Impact
Blocks: EMB-323 (Integrate limit order execution flow with existing Jumper API)
NavigationTabChangedevent ✅ deliveredselectedRouteIdstore field ✅ delivered- EIP-712 signing path ✅ reused
- Limit-order success summary ❌ deferred (confirm EMB-323 tracks this if needed)
QA Agent — 2026-07-01 (re-review of initial 2026-06-30 review)
There was a problem hiding this comment.
Requesting changes on 12 items — each requires either a code fix or an explicit acceptance comment with justification before this review is considered complete.
| # | Severity | Type | Issue / File |
|---|---|---|---|
| 1 | 🔴 Critical | Code | CI failures caused by uncommented playground _navigationTabs — 22 tests broken |
| 2 | 🟠 High | Code | mergeDefaultFormValues boolean-false corruption — partiallyFillable=false reverts to true on config update |
| 3 | 🟠 High | Code | validUntil anchored to route-fetch time, not signing time |
| 4 | 🟠 High | AC deviation | No limit-order-specific success summary (AC 13: "pair, price, validity, protocol" unimplemented) |
| 5 | 🟡 Medium | Code | ReviewButton shows "Swap Review"/"Bridge Review" label in limit mode |
| 6 | 🟡 Medium | Code | useLimitMarketRate falsy guard silently suppresses market rate when priceUSD is "0" |
| 7 | 🟡 Medium | AC deviation | Preset percentages diverge from ticket spec (+1%/+5%/+10% vs described +2%/+5%) |
| 8 | 🟡 Medium | Documentation | PR template checklist "updated public-docs" unchecked |
| 9 | 🟡 Medium | Code | isString(str: any) in createFormStore.ts — untyped parameter |
| 10 | 🟢 Low | FE8 | aria-disabled on custom chip has no keyboard effect — LimitPricePresets.tsx |
| 11 | 🟢 Low | Test gap | packages/widget/src/hooks/useLinkedLimitFields.test.ts (new) |
| 12 | 🟢 Low | Documentation | Visual showcase in PR description listed as TODO |
1. [Critical] CI failures — comment out _navigationTabs in defaultWidgetConfig.ts
packages/widget-playground/src/defaultWidgetConfig.ts has _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'] uncommented, changing the playground default from "Exchange" to tab-bar. This breaks 22 tests across 7 suites. Fix: comment the line back out. The limit tab is exercisable via the playground mode panel.
2. [High] mergeDefaultFormValues boolean-false corruption
createFormStore.ts: false || Number.isFinite(false) evaluates to false, so partiallyFillable=false is treated as "no user value" and reverts to the default (true) on any setDefaultValues call triggered by a config update. Replace with userValues[key]?.value !== undefined ? userValues[key]?.value : defaultValues[key]?.value.
3. [High] validUntil computed at route-fetch time
useRoutes.ts: Math.floor(Date.now() / 1000) + validUntilDuration is evaluated when the route query runs and re-anchored on each 60s auto-refetch. Move this to signing time, or confirm with the API team whether the routes endpoint uses validUntil at all.
4. [High] No limit-order success summary
AC 13 specifies "pair, price, validity, protocol" in the success state. The generic swap/bridge screen is shown instead. Add a LimitOrderSuccessContent component, or document explicitly in the ticket that this is deferred to EMB-323.
5. [Medium] ReviewButton label in limit mode
Add case 'limit': in getButtonText() and a corresponding button.limitReview i18n key.
6. [Medium] useLimitMarketRate falsy guard
Add a comment confirming !fromPrice is intentional when priceUSD = "0" (LiFi API convention), or use Number(priceUSD) > 0 for explicit clarity.
7. [Medium] Preset percentages
PRESETS = [0, 1, 5, 10] vs ticket "+2%, +5%". Confirm with product/design whether the implementation values are agreed.
8. [Medium] Public-docs checklist
Confirm whether WidgetEvent.NavigationTabChanged and WidgetMode = 'limit' require public documentation.
9. [Medium] isString(str: any) type
Replace with const isString = (str: unknown): str is string => typeof str === 'string' || str instanceof String.
10. [Low] aria-disabled keyboard accessibility
Add tabIndex={disabled ? -1 : 0} to EditablePriceChip in LimitPricePresets.tsx.
11. [Low] Missing unit test — useLinkedLimitFields
Add packages/widget/src/hooks/useLinkedLimitFields.test.ts covering: setSendAmount recomputes receiveAmount, setReceiveAmount updates limitPrice derivation, pair-change reset.
12. [Low] Visual showcase
Complete the TODO in the PR description before merge.
💡 Once you've addressed the items above, re-apply the "Agent Review Request" label to trigger an automated re-review.
…label, a11y, types)
|
Thanks for the review — here's the disposition of each item. Fixed (commit
|
There was a problem hiding this comment.
Requesting changes on 1 remaining blocking item — the other 11 items from the prior review have been resolved or accepted with justification.
| # | Severity | Type | Issue / File |
|---|---|---|---|
| 1 | Critical | Code | Comment out _navigationTabs in packages/widget-playground/src/defaultWidgetConfig.ts before merge |
1. [Critical] _navigationTabs still uncommented — CI broken
packages/widget-playground/src/defaultWidgetConfig.ts has _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'] still active on HEAD (326d11e1). This changes the playground default from "Exchange" to a tab-bar layout and breaks 19 Playground E2E tests and 3 Dev Smoke tests.
Fix: restore the comment — // _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'].
The developer acknowledged this in their response ("will restore before merge") but neither of the two post-review commits addressed it. All other items from the prior review are resolved or accepted.
Pre-merge hygiene also open (not hard-blockers, but should be completed):
- Item 8: PR template checklist "updated public-docs" still unchecked — check the box or add a note explaining no docs change is needed.
- Item 12: PR description still has
_TODO: attach a screen recording...— update with the recording as stated.
Once
_navigationTabsis commented out and CI passes, re-apply the "Agent Review Request" label to trigger a final automated re-review.
There was a problem hiding this comment.
Re-review complete — requesting changes on 1 remaining item. 10 of 12 previously flagged items are resolved or accepted.
| # | Severity | Type | Issue / File |
|---|---|---|---|
| 1 | 🔴 Critical | Code | Comment out _navigationTabs in packages/widget-playground/src/defaultWidgetConfig.ts |
1. [Critical] _navigationTabs still uncommented — CI is failing
packages/widget-playground/src/defaultWidgetConfig.ts HEAD (326d11e1) still has _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'] uncommented. This causes the playground to default to a tab-bar layout instead of the plain Exchange view, breaking 19 Playground E2E and 3 Dev Smoke tests.
You committed to "restoring before merge" — please comment this line out now:
// _navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'],The limit tab is exercisable via the playground mode panel without it being the global default. All other previously flagged items are cleared.
Pre-merge hygiene (no new cycle required — just complete before merging):
- Item 8: Check off or link a public-docs PR for
WidgetMode = 'limit'/WidgetEvent.NavigationTabChangedtype additions. - Item 12: Attach the screen recording to the PR description.
💡 Once you've addressed the items above, re-apply the "Agent Review Request" label to trigger an automated re-review.
Widget flows: navigation tabs, modes & per-tab UIThis doc covers the config surface added on the limit-order branch:
The two ways to pick a flowThere are two levers, and they don't stack —
Resolution order (
|
| Key | Label | variant | mode | modeOptions | requiredUI | Notes |
|---|---|---|---|---|---|---|
default |
Swap & Bridge | wide |
default |
— | — | Combined form |
private |
Private | compact |
default |
— | toAddress: true |
Forces a recipient address |
refuel |
Gas | wide |
refuel |
— | — | Gas top-up |
limit |
Limit | compact |
limit |
— | — | Limit-order flow |
swap-advanced |
Swap | wide |
split |
{ split: 'swap' } |
— | Same-chain focus |
bridge-advanced |
Bridge | wide |
split |
{ split: 'bridge' } |
— | Cross-chain focus |
swap |
Swap | — | split |
{ split: 'swap' } |
— | Public split key |
bridge |
Bridge | — | split |
{ split: 'bridge' } |
— | Public split key |
Labels come from i18n (header.*) and are overridable via the widget's translation
config.
Per-tab config layering
While a tab is active, its bundle is layered over the widget-level config, so the
rest of the widget reads tab-driven values transparently
(NavigationTabsStoreProvider). Precedence, per field:
mode,variant,modeOptions→ tab value, falling back to config (tab replaces).defaultUI,requiredUI→ shallow-merged, tab wins on collisions
({ ...config.defaultUI, ...tab.defaultUI }). Fields the tab doesn't set are
inherited from the config.
Example — a config-level defaultUI survives except where the tab overrides it:
// config.defaultUI = { transactionDetailsExpanded: true }
// active tab (e.g. 'private') sets defaultUI = { layout: 'cards' }
// → effective defaultUI = { transactionDetailsExpanded: true, layout: 'cards' }Tabs with no defaultUI/requiredUI (the swap/bridge split keys) leave the
config's values untouched.
defaultUI.layout
New defaultUI field controlling the main form layout:
defaultUI?: {
transactionDetailsExpanded?: boolean
navigationHeaderTitleNoWrap?: boolean
layout?: 'default' | 'cards' // @default 'default'
}'default'— classic chain/token selectors + single amount input.'cards'— redesigned stacked Send/Receive amount cards (AmountInputCardPair)
with inline token pills and a swap button. All configured navigation tabs set this.
You can opt a standard (tabless) widget into the card layout directly:
const config: WidgetConfig = { mode: 'default', defaultUI: { layout: 'cards' } }Reacting to tab changes
A NavigationTabChanged event fires on the first resolved tab and on every switch:
import { WidgetEvent, useWidgetEvents } from '@lifi/widget'
const widgetEvents = useWidgetEvents()
widgetEvents.on(WidgetEvent.NavigationTabChanged, ({ tab, previousTab }) => {
// tab: NavigationTabKey (now active)
// previousTab: NavigationTabKey | undefined (undefined on first render)
})Remember to clean up with .off(WidgetEvent.NavigationTabChanged, handler).
Playground
packages/widget-playground/src/defaultWidgetConfig.ts ships the advanced preset
enabled so you can see it immediately:
_navigationTabs: ['swap-advanced', 'bridge-advanced', 'limit'],Swap the array (or comment it out and set mode) to try other flows.
Which Linear task is linked to this PR?
https://linear.app/lifi-linear/issue/EMB-322
Why was it implemented this way?
Implement limit order mode as a dedicated header tab.
Extended API endpoints: lifinance/sdk#410
Visual showcase (Screenshots or Videos)
TODO: attach a screen recording of the limit price card, presets, invert toggle, and expiry/partial-fill settings.
Checklist before requesting a review